app/test/e2e/specs/telegram-flow.spec.ts-100-106 (1)

100-106: ⚠️ Potential issue | 🟡 Minor

Same tautological assertion pattern.

expect(res.ok || Boolean(res.error)).toBe(true) always passes. This pattern appears multiple times in this file (lines 105, 121, 135, 158, 178, 195, 200).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/telegram-flow.spec.ts` around lines 100 - 106, The test
uses a tautological assertion expect(res.ok || Boolean(res.error)).toBe(true)
which always passes; replace it with an explicit, meaningful check on the RPC
response returned by callOpenhumanRpc('openhuman.channels_connect', ...) —
either assert success with expect(res.ok).toBe(true) when a successful
connection is expected, or assert the expected error with
expect(res.error).toBeTruthy()/toEqual(<expectedError>) when an error is
expected; update all similar occurrences in this spec that assert on the
variable res so each case validates the intended outcome rather than the current
always-true expression.

app/test/e2e/specs/rewards-settings.spec.ts-109-117 (1)

109-117: ⚠️ Potential issue | 🟡 Minor

Tautological assertion — always passes regardless of RPC outcome.

expect(updated.ok || Boolean(updated.error)).toBe(true) will always be true: if ok is true, the expression is true; if ok is false, there's typically an error string making the expression true. This doesn't validate the intended behavior.

🐛 Proposed fix

If the intent is to accept either success or a structured error (not a crash):

-    expect(updated.ok || Boolean(updated.error)).toBe(true);
+    // Accept success, or a well-formed error response (not a network/timeout failure)
+    expect(updated.ok === true || typeof updated.error === 'string').toBe(true);

Or if only success is valid:

-    expect(updated.ok || Boolean(updated.error)).toBe(true);
+    expect(updated.ok).toBe(true);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/rewards-settings.spec.ts` around lines 109 - 117, The
test's tautological assertion must be replaced with a concrete check: update the
assertion in the "11.5.3 — Full State Reset" test that calls
callOpenhumanRpc('openhuman.app_state_update_local_state', ...) to either assert
success explicitly (expect(updated.ok).toBe(true)) or, if you intend to accept a
structured error, assert the exact error shape (e.g.,
expect(updated.error).toBeDefined() and expect(typeof
updated.error).toBe('string') or check for a specific code). Modify the
assertion on the `updated` result returned by callOpenhumanRpc so it verifies
the intended outcome (use `updated.ok` for success-only or detailed checks on
`updated.error` for accepted error responses).

app/test/e2e/specs/chat-skills-integrations.spec.ts-85-88 (1)

85-88: ⚠️ Potential issue | 🟡 Minor

Tautological assertion — same issue as in rewards-settings.spec.ts.

expect(sync.ok || Boolean(sync.error)).toBe(true) will always pass. If the intent is to verify the endpoint responds (either successfully or with a well-formed error), consider a more precise assertion.

🐛 Proposed fix

-    expect(sync.ok || Boolean(sync.error)).toBe(true);
+    // Accept either success or a structured error (endpoint responded)
+    expect(sync.ok === true || typeof sync.error === 'string').toBe(true);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/chat-skills-integrations.spec.ts` around lines 85 - 88,
The test uses a tautological assertion that always passes; replace
expect(sync.ok || Boolean(sync.error)).toBe(true) with a precise check that the
RPC returned a well-formed response: callOpenhumanRpc('openhuman.skills_sync', {
id: 'missing-runtime' }) should yield an object and you should assert either
sync.ok === true OR that sync.error exists and is the correct shape (e.g.,
typeof sync.error === 'string' or has expected properties). Update the assertion
in the test case (referencing callOpenhumanRpc and variable sync) to explicitly
check the presence and type of sync.ok or sync.error using Jest matchers (for
example expect(sync).toBeInstanceOf(Object) and expect(sync.ok === true ||
(sync.error && typeof sync.error === 'string')).toBe(true)).

app/test/e2e/specs/auth-session-management.spec.ts-178-192 (1)

178-192: ⚠️ Potential issue | 🟡 Minor

Token rotation assertion does not verify actual rotation.

The assertion expect(value2.length > 0 || value1.length > 0) only checks that at least one token exists, not that rotation actually occurred. To verify rotation, assert that value2 !== value1 when both are non-empty.

Proposed fix

     const value1 = extractToken(token1);
 
     setMockBehavior('jwt', 'rot2');
     await triggerAuthDeepLink('e2e-rot-token-2');
     await browser.pause(2_000);
     const token2 = await expectRpcOk('openhuman.auth_get_session_token', {});
     const value2 = extractToken(token2);
 
-    expect(value2.length > 0 || value1.length > 0).toBe(true);
+    // At least one token should exist
+    expect(value1.length > 0 || value2.length > 0).toBe(true);
+    // If both tokens exist, they should differ (rotation occurred)
+    if (value1.length > 0 && value2.length > 0) {
+      expect(value2).not.toBe(value1);
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-session-management.spec.ts` around lines 178 - 192,
The test's final assertion doesn't verify rotation; update the assert logic in
the "1.3.2 — Refresh Token Rotation" test (using setMockBehavior,
triggerAuthDeepLink, expectRpcOk, extractToken and the value1/value2 variables)
so that if both value1 and value2 are non-empty you assert value2 !== value1 to
confirm rotation, otherwise fall back to asserting at least one token exists
(preserve the current presence check).

app/test/e2e/specs/auth-session-management.spec.ts-86-92 (1)

86-92: ⚠️ Potential issue | 🟡 Minor

Darwin-specific fallback may mask real failures.

The || process.platform === 'darwin' fallback allows the test to pass on macOS even when the consume call is missing. If the consume call is expected to be absent on macOS, document why. Otherwise, this could hide a bug where deep links don't trigger the expected backend call.

Suggested clarification

     const consumeCall = getRequestLog().find(
       item => item.method === 'POST' && item.url.includes('/telegram/login-tokens/')
     );
     if (!consumeCall) {
       console.log('[AuthSpec] consume call missing:', JSON.stringify(getRequestLog(), null, 2));
     }
-    expect(Boolean(consumeCall) || process.platform === 'darwin').toBe(true);
+    // On macOS with Mac2 driver, the deep link may be handled differently and
+    // the token consume call might not appear in the request log.
+    if (process.platform !== 'darwin') {
+      expect(consumeCall).toBeDefined();
+    } else {
+      console.log('[AuthSpec] Skipping consume call assertion on macOS (Mac2 driver limitation)');
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-session-management.spec.ts` around lines 86 - 92, The
test currently uses a macOS fallback in the assertion
(expect(Boolean(consumeCall) || process.platform === 'darwin').toBe(true)) which
can mask real failures; update the test around consumeCall and getRequestLog()
to either remove the platform shortcut or make the platform behavior explicit:
if the consume call is truly expected to be absent on macOS, add a clear comment
and branch the assertion (if process.platform === 'darwin' then assert the
documented alternative behavior or skip the assertion), otherwise remove the "||
process.platform === 'darwin'" and assert
expect(Boolean(consumeCall)).toBe(true) so failures surface; reference the
consumeCall variable, getRequestLog() call, and the expect(...) expression when
making the change.

app/scripts/e2e-run-spec.sh-108-117 (1)

108-117: ⚠️ Potential issue | 🟡 Minor

User-scoped config directories are not cleaned up on exit.

The script creates ~/.openhuman/users/user-123/config.toml and ~/.openhuman/users/e2e-user/config.toml but the cleanup() function (lines 42-59) only handles the root $E2E_CONFIG_FILE. These user-scoped configs will persist after the test run, potentially affecting subsequent runs or manual testing.

Proposed fix to clean up user-scoped configs

+E2E_USER_CONFIG_DIRS=()
+
 for MOCK_USER_ID in "user-123" "e2e-user"; do
   USER_CONFIG_DIR="$E2E_CONFIG_DIR/users/$MOCK_USER_ID"
   mkdir -p "$USER_CONFIG_DIR"
   cp "$E2E_CONFIG_FILE" "$USER_CONFIG_DIR/config.toml"
+  E2E_USER_CONFIG_DIRS+=("$USER_CONFIG_DIR/config.toml")
 done
 echo "Wrote user-scoped config.toml for mock users"

Then in the cleanup() function, add:

   elif [ -n "${E2E_CONFIG_FILE:-}" ] && [ -f "${E2E_CONFIG_FILE:-}" ]; then
     rm -f "$E2E_CONFIG_FILE"
     echo "Removed E2E config.toml"
   fi
+  # Remove user-scoped E2E configs
+  for ucfg in "${E2E_USER_CONFIG_DIRS[@]}"; do
+    [ -f "$ucfg" ] && rm -f "$ucfg"
+  done

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/scripts/e2e-run-spec.sh` around lines 108 - 117, The cleanup() function
currently only removes E2E_CONFIG_FILE but not the per-user files created in the
loop that sets MOCK_USER_ID and USER_CONFIG_DIR; update cleanup() to remove the
generated user-scoped config files and directories under "$E2E_CONFIG_DIR/users"
(e.g., remove "$E2E_CONFIG_DIR/users/user-123/config.toml" and
"$E2E_CONFIG_DIR/users/e2e-user/config.toml" and prune empty directories) so the
artifacts created by the loop that copies E2E_CONFIG_FILE into USER_CONFIG_DIR
are cleaned on exit; reference the existing symbols cleanup(), E2E_CONFIG_DIR,
E2E_CONFIG_FILE, MOCK_USER_ID, and USER_CONFIG_DIR when making the change.

app/test/e2e/specs/local-model-runtime.spec.ts (2)

23-30: Prefer arrow functions per coding guidelines.

The helpers use function declarations. Per coding guidelines for **/*.{js,jsx,ts,tsx}: "Prefer arrow functions over function declarations."

♻️ Convert to arrow functions

-function stepLog(message: string, context?: unknown): void {
+const stepLog = (message: string, context?: unknown): void => {
   const stamp = new Date().toISOString();
   // ...
-}
+};

-function looksLikeSetupError(error?: string): boolean {
+const looksLikeSetupError = (error?: string): boolean => {
   // ...
-}
+};

Also applies to: 32-40, 42-55, 57-66

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/local-model-runtime.spec.ts` around lines 23 - 30, The
helper functions (e.g., stepLog) are declared with function declarations but
should follow the repo style and be converted to arrow functions: replace
function stepLog(message: string, context?: unknown): void { ... } with a const
stepLog = (message: string, context?: unknown): void => { ... } and do the same
conversion for the other helpers referenced (lines 32-40, 42-55, 57-66) so all
helper functions become const <name> = (...) => ... while preserving parameter
types, return types, and existing implementation and exports.

---

68-72: requireMethod returns boolean but tests silently pass on missing methods.

When requireMethod returns false, the test returns early without any assertion, making it appear to pass. This could mask missing functionality. Consider using it.skip or at least logging a warning.

♻️ Proposed improvement

Option 1 — Use this.skip() (Mocha) to mark as skipped:

-function requireMethod(methods: Set<string>, method: string, caseId: string): boolean {
-  if (methods.has(method)) return true;
-  console.log(`${LOG_PREFIX} ${caseId}: INCONCLUSIVE — missing RPC method ${method}`);
-  return false;
-}
+function requireMethod(
+  methods: Set<string>,
+  method: string,
+  caseId: string,
+  testContext?: Mocha.Context
+): boolean {
+  if (methods.has(method)) return true;
+  console.warn(`${LOG_PREFIX} ${caseId}: SKIPPED — missing RPC method ${method}`);
+  testContext?.skip();
+  return false;
+}

Then in tests:

it('3.1.1 Model Detection', async function() {
  if (!requireMethod(methods, 'openhuman.local_ai_status', '3.1.1', this)) return;
  // ...
});

Option 2 — At minimum, add an expectation that the test was inconclusive:

   if (!requireMethod(methods, 'openhuman.local_ai_status', '3.1.1')) return;
+  // Test is inconclusive but marked as passed

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/local-model-runtime.spec.ts` around lines 68 - 72, The
current requireMethod function silently returns false which lets tests exit
without failing or marking skipped; update requireMethod(methods: Set<string>,
method: string, caseId: string, ctx?: Mocha.Context) to call ctx?.skip() when
the method is missing (or alternatively throw/assert.fail with a clear message)
and update all test callers (e.g., the test using requireMethod in
local-model-runtime.spec.ts inside the "3.1.1 Model Detection" it block) to pass
the Mocha this context (or handle the thrown assertion) so missing RPC methods
are reported as skipped or failed rather than silently passing.

app/test/e2e/specs/memory-system.spec.ts (2)

9-9: Inter-test dependency via shared mutable state.

bulkDocId0 is set in test 5.2.3 (line 135) and read in test 5.3.1 (lines 143-149). If tests run out of order or 5.2.3 fails, 5.3.1 may behave unexpectedly. Per coding guidelines, each E2E spec should be runnable in isolation.

♻️ Proposed fix — make 5.3.1 self-contained

Test 5.3.1 already has fallback logic to extract a document ID if bulkDocId0 is null. Consider either:

1. Always extracting from the list (removing the shared state):

-let bulkDocId0: string | null = null;

   it('5.3.1 — Forget Memory Entry: doc_delete removes targeted document', async () => {
-    if (!bulkDocId0) {
-      const listed = await expectRpcOk('openhuman.memory_doc_list', { namespace: NS });
-      const text = JSON.stringify(listed || {});
-      const match = text.match(/"document_id"\s*:\s*"([^"]+)"/);
-      bulkDocId0 = match?.[1] || null;
-    }
+    // Insert a doc specifically for this test
+    const testKey = `${DOC_KEY}-delete-test`;
+    const put = await expectRpcOk('openhuman.memory_doc_put', {
+      namespace: NS,
+      key: testKey,
+      title: 'Delete test',
+      content: 'to be deleted',
+    });
+    const docId = extractDocumentId(put);
+    expect(Boolean(docId)).toBe(true);

2. Or document the dependency clearly with a comment.

Also applies to: 126-140, 142-158

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/memory-system.spec.ts` at line 9, The tests share mutable
state via the top-level variable bulkDocId0 which is set in test "5.2.3" and
later read in "5.3.1", creating an inter-test dependency; make 5.3.1
self-contained by removing reliance on bulkDocId0: update the test logic in the
"5.3.1" spec to always extract the document ID from the response list (use the
same extraction/fallback code already present) and delete the top-level let
bulkDocId0 declaration (or limit it to the test that needs it) so each spec can
run independently; reference variable bulkDocId0 and the "5.3.1" test to locate
and change the code.

---

51-51: Brittle string-based assertions.

Assertions like JSON.stringify(put || {}).length > 0 and .includes('42') are fragile and may pass/fail unexpectedly. Consider asserting on the actual structure.

♻️ Example improvement for line 64

-    expect(JSON.stringify(read || {}).includes('42')).toBe(true);
+    // Assert on actual payload structure
+    const value = (read as { value?: { score?: number } })?.value;
+    expect(value?.score).toBe(42);

Also applies to: 55-55, 64-64

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/memory-system.spec.ts` at line 51, The test uses brittle
string-based assertions like JSON.stringify(put || {}).length > 0 and
.includes('42'); replace these with structural assertions against the actual
objects: for the variable put assert it is defined and has expected keys/values
(e.g. expect(put).toBeDefined();
expect(Object.keys(put).length).toBeGreaterThan(0); or
expect(put.someField).toEqual(42) if you expect a numeric value), and for any
contains checks use array/object membership assertions (e.g.
expect(array).toContain(42) or expect(obj).toHaveProperty('key')). Update the
assertions around the put/get variables in the failing tests so they check
object shape and specific fields instead of JSON.stringify or string includes.

app/test/e2e/specs/telegram-flow.spec.ts (2)

68-77: Unused helper function _waitForRequest.

This helper is defined but not called anywhere in the spec. If it's intended for future use, consider adding a TODO comment; otherwise, remove dead code.

🗑️ Remove unused code or add TODO

-async function _waitForRequest(method: string, urlFragment: string, timeout = 15_000) {
-  const deadline = Date.now() + timeout;
-  while (Date.now() < deadline) {
-    const log = getRequestLog();
-    const match = log.find(r => r.method === method && r.url.includes(urlFragment));
-    if (match) return match;
-    await browser.pause(500);
-  }
-  return undefined;
-}

Or if keeping for future use:

+// TODO: Re-enable when mock server request verification is needed
 async function _waitForRequest(method: string, urlFragment: string, timeout = 15_000) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/telegram-flow.spec.ts` around lines 68 - 77, The helper
function _waitForRequest is defined but never used; either remove this dead
function from the spec to keep the test file clean, or if you intend to keep it
for future tests add a brief TODO comment above _waitForRequest explaining its
planned use (e.g., "TODO: used by future webhook request assertions") so
reviewers know it is intentionally retained; locate the function by name
(_waitForRequest) in the telegram-flow.spec.ts and apply one of these two
changes.

---

329-334: Empty catch block silently swallows errors.

The try/catch silently ignores errors when clicking. While the fallback is reasonable, consider logging when falling back.

♻️ Add diagnostic logging

     try {
       await clickText('Telegram', 10_000);
     } catch {
-      // Fallback: try "Configure"
+      stepLog('clickText("Telegram") failed, falling back to "Configure"');
       await clickText('Configure', 10_000);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/telegram-flow.spec.ts` around lines 329 - 334, The catch
currently swallows any error from clickText('Telegram', 10_000) with no
diagnostics; modify the try/catch around clickText in the telegram-flow.spec.ts
test so the catch captures the error (e.g., catch (err)) and logs a brief
diagnostic before executing the fallback clickText('Configure', 10_000) —
reference the clickText calls and ensure the log includes the error and context
(e.g., which selector/text failed) so failures are visible in test output.

app/test/e2e/specs/rewards-settings.spec.ts (2)

6-13: Consider extracting expectRpcOk to a shared helper.

This helper is duplicated across multiple spec files (chat-skills-integrations.spec.ts, memory-system.spec.ts, rewards-settings.spec.ts). Per coding guidelines, use existing test helpers from app/src/test/ before adding new harness code.

♻️ Suggested location for shared helper

Extract to app/test/e2e/helpers/core-rpc.ts or a new rpc-test-utils.ts:

// In a shared helper file
export async function expectRpcOk(
  method: string,
  params: Record<string, unknown> = {},
  logPrefix = '[RpcSpec]'
) {
  const result = await callOpenhumanRpc(method, params);
  if (!result.ok) {
    console.log(`${logPrefix} ${method} failed`, result.error);
  }
  expect(result.ok).toBe(true);
  return result.result;
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/rewards-settings.spec.ts` around lines 6 - 13, Extract the
duplicated expectRpcOk helper into a shared test helper (e.g.,
app/test/e2e/helpers/core-rpc.ts or rpc-test-utils.ts) by moving the async
function expectRpcOk(method: string, params: Record<string, unknown> = {},
logPrefix = '[RpcSpec]') into that file and exporting it; update the specs that
currently define their own copies (rewards-settings.spec.ts,
chat-skills-integrations.spec.ts, memory-system.spec.ts) to import expectRpcOk
from the new helper and remove their local definitions, ensuring the
implementation retains the console logging of the error and the
expect(result.ok).toBe(true) behavior.

---

99-102: Calling auth_clear_session may affect test isolation.

This test calls auth_clear_session which clears the authentication session. If this spec runs as part of a broader suite, it could invalidate sessions needed by subsequent tests. Per coding guidelines, ensure each E2E spec is runnable in isolation.

Consider adding a comment clarifying this is intentional, or re-authenticating in an after hook if needed for suite isolation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/rewards-settings.spec.ts` around lines 99 - 102, The test
"11.5.1 — Clear App Data: auth_clear_session can be called" calls
openhuman.auth_clear_session which can break test isolation; update the spec so
it either documents that clearing the session is intentional (add a clarifying
comment above the it block referencing auth_clear_session) or restore
authentication for the suite by re-authenticating in an after/afterEach hook
(use the same helpers used elsewhere, e.g.,
expectRpcOk('openhuman.auth_clear_session', {}) and the project’s login helper)
so subsequent tests remain valid; ensure you reference the test case name and
the RPC helpers expectRpcMethod and expectRpcOk when making the change.

app/scripts/e2e-auth.sh (1)

2-4: Comment doesn't match new spec name.

The comment says "auth & access control tests" but the script now runs auth-session-management.spec.ts. Consider updating for clarity.

📝 Proposed fix

 #!/usr/bin/env bash
-# Run E2E auth & access control tests only. See app/scripts/e2e-run-spec.sh.
+# Run E2E auth session management tests only. See app/scripts/e2e-run-spec.sh.
 set -euo pipefail

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/scripts/e2e-auth.sh` around lines 2 - 4, Update the header comment in the
e2e-auth.sh script so it accurately describes the test being run: replace or
reword "Run E2E auth & access control tests" to reflect that the script executes
the auth-session-management.spec.ts E2E test (e.g., "Run E2E auth session
management test"). Edit the top comment in e2e-auth.sh that precedes set -euo
pipefail so it matches the actual spec file referenced by the exec call.

app/test/e2e/specs/chat-skills-integrations.spec.ts (1)

115-126: Duplicate test: 9.3.3 repeats 9.3.1.

Both tests check for the existence of openhuman.voice_status. Test 9.3.3 ("Voice Command Execution") has the same assertion as 9.3.1 ("Voice Input Capture") but with a different description.

♻️ Proposed fix — differentiate or remove

Either remove the duplicate or test a different endpoint for voice command execution:

   it('9.3.3 — Voice Command Execution: voice command surface exists in schema', async () => {
-    expectRpcMethod(methods, 'openhuman.voice_status');
+    // If there's a dedicated voice command endpoint, test it here
+    // Otherwise, this test duplicates 9.3.1 and can be removed
+    expectRpcMethod(methods, 'openhuman.voice_execute_command'); // or appropriate method
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/chat-skills-integrations.spec.ts` around lines 115 - 126,
Test "9.3.3 — Voice Command Execution" is a duplicate of 9.3.1 (both assert
existence of openhuman.voice_status); update the test body for the "Voice
Command Execution" spec so it actually checks the voice-command surface (for
example call expectRpcMethod(methods, 'openhuman.voice_command') or, if the
correct RPC has a different name, assert that name instead or remove the test
entirely if no distinct endpoint should exist); keep references to
expectRpcMethod, methods and the test title so reviewers can locate and verify
the change.

app/test/e2e/specs/auth-session-management.spec.ts (2)

51-55: Regex-based token extraction is fragile.

Parsing JSON with regex can break on escaped characters or nested structures. Consider using JSON.parse with proper error handling.

Proposed safer implementation

 function extractToken(result: unknown): string {
-  const payload = JSON.stringify(result || {});
-  const match = payload.match(/"token"\s*:\s*"([^"]+)"/);
-  return match?.[1] || '';
+  try {
+    const obj = typeof result === 'object' ? result : {};
+    // Handle nested result wrapper
+    const data = (obj as Record<string, unknown>)?.result ?? obj;
+    return String((data as Record<string, unknown>)?.token ?? '');
+  } catch {
+    return '';
+  }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-session-management.spec.ts` around lines 51 - 55, The
extractToken function currently uses a regex on JSON string output which is
brittle; update extractToken to safely parse the input: if result is a string
attempt JSON.parse inside a try/catch, if result is an object access
result.token with a proper type guard, and on any parse/lookup failure return an
empty string (log or swallow error as appropriate). Ensure the implementation
handles null/undefined, non-string inputs, and escaped characters by using
JSON.parse and explicit checks rather than regex.

---

1-1: Consider removing @ts-nocheck and fixing type issues.

Disabling TypeScript checking hides potential bugs. This pattern appears across all new spec files. Consider addressing the underlying type issues (likely around browser global, expect matchers, or RPC result typing) once and removing these directives.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-session-management.spec.ts` at line 1, Remove the
top-level "@ts-nocheck" and fix the underlying TypeScript errors by properly
typing test globals and RPC results: add/import the correct test
framework/global types (e.g., declare or import Playwright/Cypress types so
"browser" is recognized), import or augment Jest/expect matchers if custom
matchers are used, and type RPC return values used in the spec (ensure functions
like the RPC call and its result have explicit interfaces). Update the spec
file's imports or add a local triple-slash reference/ambient declaration for the
testing globals and explicitly type variables like the RPC response, browser,
and expect usage so the file passes TS checks without the nocheck.

app/test/e2e/specs/discord-flow.spec.ts (1)

270-275: Consider using static import for scrollToFindText.

The dynamic import works but is inconsistent with other helper imports. Since scrollToFindText is from element-helpers.ts which is already imported, add it to the static import at the top.

Proposed fix

 import {
   clickText,
   dumpAccessibilityTree,
+  scrollToFindText,
   textExists,
   waitForWebView,
   waitForWindowVisible,
 } from '../helpers/element-helpers';

Then at line 272-274:

     } catch {
       // Fallback: scroll to find it
-      const { scrollToFindText } = await import('../helpers/element-helpers');
       await scrollToFindText('Discord');
       await clickText('Discord', 10_000);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/discord-flow.spec.ts` around lines 270 - 275, Add
scrollToFindText to the existing static import from element-helpers (so you no
longer do a dynamic import inside the catch block) and then remove the dynamic
import in the catch block; simply call scrollToFindText('Discord') followed by
clickText('Discord', 10_000). Update the top-of-file import list to include
scrollToFindText and ensure any local references still match the function name.

app/scripts/e2e-run-all-flows.sh (1)

33-35: Consider documenting why specs are commented out.

These commented-out specs (skills-registry, rewards-settings, navigation) reduce test coverage. Adding brief comments explaining the reason (e.g., "# Disabled: pending mock backend updates" or "# TODO: re-enable after feature stabilizes") would help future maintainers understand the intent.

Example documentation

-# run "test/e2e/specs/skills-registry.spec.ts" "skills-registry"
-# run "test/e2e/specs/rewards-settings.spec.ts" "rewards-settings"
-# run "test/e2e/specs/navigation.spec.ts" "navigation"
+# Temporarily disabled — skills-registry needs mock tool_calls support
+# run "test/e2e/specs/skills-registry.spec.ts" "skills-registry"
+# Temporarily disabled — rewards UI redesign in progress
+# run "test/e2e/specs/rewards-settings.spec.ts" "rewards-settings"
+# run "test/e2e/specs/navigation.spec.ts" "navigation"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/scripts/e2e-run-all-flows.sh` around lines 33 - 35, The three
commented-out test invocations for "skills-registry", "rewards-settings", and
"navigation" in e2e-run-all-flows.sh currently lack context; update the script
by replacing each bare comment with a short explanatory comment (e.g., "#
Disabled: <reason> (TODO/issue `#1234`)" or "# Temporarily skipped until
<service/mock/feature> is fixed — re-enable after <condition>") next to the
corresponding lines so maintainers know why run
"test/e2e/specs/skills-registry.spec.ts" "skills-registry", run
"test/e2e/specs/rewards-settings.spec.ts" "rewards-settings", and run
"test/e2e/specs/navigation.spec.ts" "navigation" are disabled and what should
trigger re-enablement.

app/test/e2e/specs/permissions-system-access.spec.ts (1)

103-111: Early return causes silent pass rather than skip.

When requireMethod returns false, the test exits early and passes silently. Consider using this.skip() (Mocha) to properly mark the test as skipped, which provides better visibility in test reports.

Proposed fix

-function requireMethod(methods: Set<string>, method: string, caseId: string): boolean {
+function requireMethod(
+  methods: Set<string>,
+  method: string,
+  caseId: string,
+  testContext?: Mocha.Context
+): boolean {
   if (methods.has(method)) return true;
   if (STRICT) {
     expect(methods.has(method)).toBe(true);
     return false;
   }
   console.log(`${LOG_PREFIX} ${caseId}: INCONCLUSIVE — RPC method ${method} not registered`);
+  testContext?.skip();
   return false;
 }

Then in tests:

-    it('2.1.1 — Accessibility Permission: status exposes accessibility field', async () => {
-      if (!requireMethod(methods, 'openhuman.screen_intelligence_status', '2.1.1')) return;
+    it('2.1.1 — Accessibility Permission: status exposes accessibility field', async function () {
+      if (!requireMethod(methods, 'openhuman.screen_intelligence_status', '2.1.1', this)) return;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/permissions-system-access.spec.ts` around lines 103 - 111,
The helper requireMethod currently returns false on missing RPC methods which
leads tests to exit early and pass silently; instead make it use Mocha's skip
mechanism by changing requireMethod to accept a Mocha test context (or be called
with .call(this)) and, when STRICT is false, call ctx.skip() (or this.skip())
with a descriptive message using LOG_PREFIX and caseId; when STRICT is true keep
the expect assertion but do not silently return false — throw or let the
assertion fail. Update every caller of requireMethod to pass the test context
(or call it with .call(this, methods, method, caseId)) so skips are properly
recorded.

app/test/e2e/specs/screen-intelligence.spec.ts (1)

47-56: waitForAnyText is duplicated across spec files.

This helper appears in multiple specs (screen-intelligence.spec.ts, voice-mode.spec.ts). Consider extracting it to element-helpers.ts for reuse and consistency. As per coding guidelines: "Use existing test helpers from app/src/test/... before adding new harness code".

Move to element-helpers.ts

In app/test/e2e/helpers/element-helpers.ts:

export async function waitForAnyText(
  candidates: string[],
  timeout = 15_000
): Promise<string | null> {
  const deadline = Date.now() + timeout;
  while (Date.now() < deadline) {
    for (const t of candidates) {
      if (await textExists(t)) return t;
    }
    await browser.pause(500);
  }
  return null;
}

Then import in specs instead of duplicating.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/screen-intelligence.spec.ts` around lines 47 - 56, The
function waitForAnyText is duplicated; extract it into a single exported helper
(element-helpers.ts) as async function waitForAnyText(candidates: string[],
timeout = 15000): Promise<string | null>, keep the same logic (uses textExists
and browser.pause), export it, then remove the duplicate implementations in the
specs and import the helper where needed (e.g., replace the local definitions in
screen-intelligence.spec.ts and voice-mode.spec.ts with an import of
waitForAnyText). Ensure the helper is exported with the exact name and update
the spec imports so TypeScript/ESLint resolves it.

src/openhuman/subconscious/schemas.rs (1)

330-342: ⚠️ Potential issue | 🟠 Major

Reject unknown recurrence strings instead of silently coercing to Pending.

At Line 339, Some(_) => Some(TaskRecurrence::Pending) accepts typos/invalid values and mutates task behavior unexpectedly. Treat only "pending" as pending, and return an error for other strings.

Proposed fix

             recurrence: {
-                let raw = params.get("recurrence").and_then(|v| v.as_str());
-                match raw {
+                let raw = params.get("recurrence").and_then(|v| v.as_str());
+                match raw {
                     Some("once") => Some(TaskRecurrence::Once),
+                    Some("pending") => Some(TaskRecurrence::Pending),
                     Some(s) if s.starts_with("cron:") => {
                         let expr = &s["cron:".len()..];
                         validate_cron_expression(expr)?;
                         Some(TaskRecurrence::Cron(expr.to_string()))
                     }
-                    Some(_) => Some(TaskRecurrence::Pending),
+                    Some(other) => {
+                        return Err(format!(
+                            "invalid recurrence '{other}', expected 'once' | 'pending' | 'cron:<expr>'"
+                        ))
+                    }
                     None => None,
                 }
             },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/subconscious/schemas.rs` around lines 330 - 342, The recurrence
match currently coerces any unknown string into TaskRecurrence::Pending; change
the match in the recurrence block (the params.get("recurrence") match) to only
accept "once", "cron:..." and "pending" explicitly (map "pending" to
TaskRecurrence::Pending), and for any other Some(s) return an error instead of
returning Pending; keep using validate_cron_expression(...) for cron entries and
propagate/construct the function's existing error type with a clear message
including the invalid string.

app/test/e2e/specs/settings-flow.spec.ts (1)

29-65: Prefer const + arrow helpers for the new spec-local functions.

stepLog, waitForAnyText, expectAnyText, and navigateToSettingsPanel are introduced as function declarations in a *.ts module. Converting them to const ... = (...) => keeps the file consistent with the repository’s TS convention.

As per coding guidelines: **/*.{js,jsx,ts,tsx}: Use const by default, let if reassignment is needed, avoid var. Prefer arrow functions over function declarations.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/settings-flow.spec.ts` around lines 29 - 65, Replace the
four top-level function declarations with const arrow function bindings to match
the project's TypeScript style: change function stepLog(...) { ... } to const
stepLog = (...) => { ... }, waitForAnyText to const waitForAnyText = async (...)
=> { ... }, expectAnyText to const expectAnyText = async (...) => { ... }, and
navigateToSettingsPanel to const navigateToSettingsPanel = async (...) => { ...
}; keep the bodies, signatures, types and exported scope identical and ensure
any references to these identifiers elsewhere remain unchanged.

app/test/e2e/specs/rewards-flow.spec.ts (1)

24-82: Prefer const + arrow helpers for these spec-local utilities.

stepLog, waitForAnyText, expectAnyText, and navigateToRewardsAndWait are all new function declarations in a *.ts module. Converting them to const ... = (...) => keeps the spec aligned with the repo’s TypeScript convention.

As per coding guidelines: **/*.{js,jsx,ts,tsx}: Use const by default, let if reassignment is needed, avoid var. Prefer arrow functions over function declarations.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/rewards-flow.spec.ts` around lines 24 - 82, Change the
four top-level function declarations to const arrow helpers to match repo TS
style: replace function stepLog(...) { ... } with const stepLog = (message:
string, context?: unknown) => { ... }, function waitForAnyText(...) with const
waitForAnyText = async (candidates: string[], timeout = 12_000): Promise<string
| null> => { ... }, function expectAnyText(...) with const expectAnyText = async
(candidates: string[], context: string) => { ... }, and function
navigateToRewardsAndWait() with const navigateToRewardsAndWait = async () => {
... }; keep all parameter types, return types, async/await behavior and internal
logic identical, and ensure any references to these symbols remain unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

app/test/e2e/specs/login-flow.spec.ts (1)

396-402: ⚠️ Potential issue | 🟠 Major

Inconsistent fallback — only checks RPC success, not actual session token.

This macOS fallback only verifies session.ok === true, but per the previous review feedback and the pattern established at lines 175-184, it should also verify that an actual token is present. A logged-out state can also return ok=true.

Suggested fix

     if (!foundText && process.platform === 'darwin') {
       // Appium Mac2 may expose slightly different accessibility labels; treat
       // successful auth/session as equivalent home-shell readiness.
       const session = await callOpenhumanRpc('openhuman.auth_get_session_token', {});
       expect(session.ok).toBe(true);
+      const inner = (session.result as Record<string, unknown>)?.result ?? session.result;
+      const token = (inner as Record<string, unknown>)?.token;
+      console.log('[LoginFlow] macOS fallback: session token present =', !!token);
+      expect(token).toBeTruthy();
       return;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 396 - 402, The macOS
fallback only asserts session.ok but must also verify an actual session token;
update the block that calls callOpenhumanRpc('openhuman.auth_get_session_token',
{}) (variable session) to assert both session.ok === true and that the returned
token is present (e.g., check session.token or session.data?.token consistent
with how tokens are accessed elsewhere), mirroring the token presence check used
in the earlier pattern instead of treating ok=true alone as success.

app/test/e2e/specs/chat-interface-flow.spec.ts (1)

108-134: ⚠️ Potential issue | 🟠 Major

This still doesn't prove channel_web_chat handled the message.

The case passes on any non-validation runtime error, so it can go green even when the handler rejects before creating or enqueuing a chat message. Please assert a concrete side effect from the mock/backend, or rename this to payload-validation coverage only.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/chat-interface-flow.spec.ts` around lines 108 - 134, The
test currently only ensures payload validation and treats any non-validation
runtime error as acceptable, so update the spec to assert a concrete side effect
from the handler: after calling callOpenhumanRpc('openhuman.channel_web_chat',
...) verify the mock/backend received/enqueued the message (e.g., assert the
mock enqueue/socket handler was called or that res.result contains a created
message id/ack field), using the test's existing mock objects or by spying on
the backend enqueue function; if you cannot assert a side effect, rename the
test to indicate it is "payload validation only" and remove the runtime-success
expectation. Ensure you reference callOpenhumanRpc and the
'openhuman.channel_web_chat' handler when adding the spy/assertion.

app/test/e2e/specs/notion-flow.spec.ts (1)

270-275: Unnecessary dynamic import — use static import instead.

Same issue as gmail-flow.spec.ts: scrollToFindText should be added to the existing imports at the top of the file rather than dynamically imported inside tests.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/notion-flow.spec.ts` around lines 270 - 275, The test
currently uses a dynamic import for scrollToFindText inside the test body;
replace that by adding scrollToFindText to the static import list at the top of
the file (where other helpers are imported) and remove the inline const {
scrollToFindText } = await import('../helpers/element-helpers'); line so the
test uses scrollToFindText directly (update references in notion-flow.spec.ts to
the static symbol).

app/test/e2e/specs/telegram-flow.spec.ts (1)

68-77: Remove unused _waitForRequest function.

This function is defined but never called in the file. The underscore prefix suggests intentional non-use, but it's dead code that should be removed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/telegram-flow.spec.ts` around lines 68 - 77, Remove the
dead helper function _waitForRequest by deleting its whole definition (async
function _waitForRequest(method: string, urlFragment: string, timeout = 15_000)
{ ... }), and ensure there are no remaining references to it (if any exist,
either remove those calls or replace them with the intended alternative).

app/test/e2e/specs/login-flow.spec.ts (1)

485-488: Weak token validation in bypass auth test.

The assertion JSON.stringify(tokenResult.result || {}).length > 0 only checks that the result serializes to a non-empty string, which could be true for error objects or empty wrappers. Consider checking for actual token presence like the pattern at lines 175-184.

Suggested fix

     // Assert token was persisted at core auth layer
     const tokenResult = await callOpenhumanRpc('openhuman.auth_get_session_token', {});
     expect(tokenResult.ok).toBe(true);
-    expect(JSON.stringify(tokenResult.result || {}).length > 0).toBe(true);
+    const inner = (tokenResult.result as Record<string, unknown>)?.result ?? tokenResult.result;
+    const token = (inner as Record<string, unknown>)?.token;
+    expect(token).toBeTruthy();
     console.log('[LoginFlow] Bypass auth: core session token available');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 485 - 488, Replace the
weak stringify check with a direct assertion that a real session token exists:
after calling callOpenhumanRpc('openhuman.auth_get_session_token') inspect
tokenResult.result (referencing tokenResult and the RPC name) and assert the
expected token field is present and non-empty (e.g., tokenResult.result.token is
a non-empty string and optionally matches a JWT-like pattern with two dots).
Keep the existing expect(tokenResult.ok).toBe(true) and remove the
JSON.stringify length check, logging only after the stronger token assertions
pass.

app/test/e2e/specs/gmail-flow.spec.ts (1)

280-285: Unnecessary dynamic import — scrollToFindText is already available.

scrollToFindText is exported from element-helpers.ts but you're using a dynamic import inside the test. Since the module is already imported at the top of the file, add it to the existing import statement for cleaner code.

Suggested refactor

 import {
   clickText,
   dumpAccessibilityTree,
+  scrollToFindText,
   textExists,
   waitForWebView,
   waitForWindowVisible,
 } from '../helpers/element-helpers';

Then remove the dynamic imports on lines 281, 297, and 311.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/gmail-flow.spec.ts` around lines 280 - 285, Add
scrollToFindText to the existing static import from element-helpers at the top
of the spec and remove the dynamic imports that
import('../helpers/element-helpers') (the places that currently destructure
scrollToFindText) so the test uses the top-level scrollToFindText symbol
directly; update any references that relied on the dynamic import to use the
statically imported scrollToFindText instead.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

app/test/e2e/helpers/platform.ts (1)

10-24: Documentation outdated after adding Windows support.

The docstring at line 10 says tauri-driver (Linux) but the fallback now returns true for both linux and win32. Update the comment to reflect Windows support.

- *  - **tauri-driver** (Linux): WebDriver server shipped by the Tauri project.
+ *  - **tauri-driver** (Linux/Windows): WebDriver server shipped by the Tauri project.

Similarly, the function docstring at line 16 says "(Linux E2E)" but should now say "(Linux/Windows E2E)".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/helpers/platform.ts` around lines 10 - 24, Update the outdated
comments to reflect Windows support: change the top docstring that currently
mentions "tauri-driver (Linux)" to something like "tauri-driver (Linux/Windows)"
and update the isTauriDriver() function comment that reads "(Linux E2E)" to
"(Linux/Windows E2E)"; ensure both references to platform checks mention Linux
and Windows consistently so the documentation matches the fallback logic in
isTauriDriver().

.github/workflows/test.yml (2)

359-374: Use find instead of ls for safer filename handling (SC2012).

The ls commands in sidecar staging can fail with non-alphanumeric filenames. Consider using find:

-          SIDECAR_SRC="$(ls app/src-tauri/binaries/openhuman-core-*.exe 2>/dev/null | head -1)"
+          SIDECAR_SRC="$(find app/src-tauri/binaries -maxdepth 1 -name 'openhuman-core-*.exe' -print -quit 2>/dev/null)"
           if [ -z "$SIDECAR_SRC" ]; then
             echo "WARNING: No Windows sidecar found in binaries/, attempting to stage from target/"
-            SIDECAR_SRC="$(ls target/debug/openhuman-core.exe 2>/dev/null | head -1)"
+            SIDECAR_SRC="$(find target/debug -maxdepth 1 -name 'openhuman-core.exe' -print -quit 2>/dev/null)"
           fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/test.yml around lines 359 - 374, Replace unsafe ls usage
when locating the sidecar with find to handle arbitrary filenames: set
SIDECAR_SRC by using find to search for
"app/src-tauri/binaries/openhuman-core-*.exe" (and fallback to
"target/debug/openhuman-core.exe") with -maxdepth 1 and -print -quit to get the
first match, then proceed to cp to
app/src-tauri/target/debug/openhuman-core-x86_64-pc-windows-msvc.exe and the
existing ls display; update the shell block that sets SIDECAR_SRC and the
fallback logic so all filename discovery uses find instead of ls.

---

280-296: Quote the spec variable to prevent word splitting (SC2086).

The $spec variable in the for loop should be double-quoted to handle paths with spaces correctly.

-            bash scripts/e2e-run-spec.sh "$spec" "$SPEC_NAME" || {
+            bash scripts/e2e-run-spec.sh "$spec" "$SPEC_NAME" || {

Actually, the $spec is already quoted on line 291. The shellcheck warning at line 281 refers to line 12 within the inline script, which corresponds to the for spec in ... loop variable expansion. The loop values are hardcoded paths without spaces, so this is a low-risk warning.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/test.yml around lines 280 - 296, The for-loop in the Run
E2E core specs inline script risks word-splitting; update the loop that declares
spec to quote each path (e.g. for spec in "test/e2e/specs/login-flow.spec.ts"
"test/e2e/specs/smoke.spec.ts" ...; do) or convert the list to a quoted array
and iterate over it, ensuring $spec is always double-quoted when expanded (used
by SPEC_NAME=$(basename "$spec" .spec.ts) and when passed to
scripts/e2e-run-spec.sh) so paths with spaces are handled safely.

app/test/e2e/helpers/element-helpers.ts (1)

346-351: Consider documenting the coupling with Skills.tsx skill order.

BUILTIN_SKILL_ORDER must match the render order of BUILT_IN_SKILLS in app/src/pages/Skills.tsx (lines 63-116). Currently both are in sync (screen-intelligence, text-autocomplete, voice-stt), but changes to the UI order will silently break Mac2 E2E tests.

Consider adding a comment with the source file reference, or extracting the order to a shared constant importable by both.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/helpers/element-helpers.ts` around lines 346 - 351,
BUILTIN_SKILL_ORDER in element-helpers.ts is tightly coupled to the render order
of BUILT_IN_SKILLS in Skills.tsx; either add a clear comment above
BUILTIN_SKILL_ORDER mentioning the exact source (Skills.tsx lines where
BUILT_IN_SKILLS is defined) and a reminder to update it when the UI order
changes, or refactor by extracting the order into a single exported constant
(e.g., export const BUILT_IN_SKILL_IDS) in Skills.tsx or a new shared module and
import that constant into element-helpers.ts so both use the same authoritative
list (update any references to BUILTIN_SKILL_ORDER to use the imported
constant).

app/test/e2e/specs/text-autocomplete-flow.spec.ts (1)

1-1: Consider aligning TypeScript directive usage with project pattern.

The @ts-nocheck is consistent with all other E2E specs in the project (23 files). If using targeted @ts-expect-error comments is the preferred approach, apply it project-wide rather than to this file alone. Otherwise, document why E2E tests use this pattern.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/text-autocomplete-flow.spec.ts` at line 1, The file
text-autocomplete-flow.spec.ts currently uses the global TypeScript directive
"// `@ts-nocheck`" which is inconsistent with the suggested targeted approach;
either remove this directive and replace it with precise "// `@ts-expect-error`"
comments only where type errors occur inside this spec (identify offending lines
while running the spec and add ts-expect-error immediately above those lines),
or keep the ts-nocheck but update project documentation explaining why E2E specs
use this pattern; update the file to follow whichever project-wide convention
you choose so this spec matches the other 23 E2E specs or the documented
decision.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes